-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
⬆️ UPGRADE: sqlalchemy v1.4 (v2 API) #114
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #114 +/- ##
===========================================
- Coverage 99.68% 99.56% -0.13%
===========================================
Files 7 7
Lines 1585 1601 +16
===========================================
+ Hits 1580 1594 +14
- Misses 5 7 +2
Continue to review full report at Codecov.
|
1b9a057
to
848a95b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything great here 👍🏽 I just have one question out of curiosity but this is ready to merge for me.
session.execute(text("COMMIT")) | ||
session.execute(text("VACUUM")) | ||
# ensure sqlalchemy knows to open a new transaction for the next execution | ||
session.commit() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the difference between session.execute(text("COMMIT"))
and session.commit()
and why is it necessary to make it differently here? (I imagine it has to do with the comment of different transactions but why)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The difference is that this direct execution bypasses sqlalchemy’s auto-transaction mechanics, and stops it from starting a new transaction before executing the vacuum (which cannot be executed within a transaction)
Cheers! |
Currently based on top of #113, supersedes #108Following migration: https://docs.sqlalchemy.org/en/14/changelog/migration_20.html
SQLALCHEMY_WARN_20
environmental variablefuture=True
for engine and session creation (V1 -> v2 API)query
->select
(V1 -> v2 API)models.py
->database.py
(models is too generic does not describe the module's purpose)get_session
->database.py
(this method can be independent of the container)I noted the
count()
method is now a bit more complex, but this is explained in: sqlalchemy/sqlalchemy#6794A few things to double-check:
A number of queries used
with_entities
, but I was unclear why this was necessary,i.e. I changed
query(Obj)...with_entities(Obj.hashkey)
toselect(Obj.hashkey)..
@giovannipizzi any reason why this would be an issue?Vacuum now fails, since it is in a transaction. I've added a workaround, but checked on this in: How to perform a vacuum? sqlalchemy/sqlalchemy#6959 (comment)